Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bump CONTROLLER_TOOLS_VERSION to 0.14.0 #669

Merged
merged 1 commit into from
Dec 9, 2024
Merged

Bump CONTROLLER_TOOLS_VERSION to 0.14.0 #669

merged 1 commit into from
Dec 9, 2024
+9 −15

Conversation

stuggi
Copy link
Contributor

@stuggi stuggi commented Dec 9, 2024

bump controller tools to the version which bumps k8s deps to v0.29.0, which aligns with the version in the operators.

@openshift-ci openshift-ci bot requested review from abays and dprince December 9, 2024 06:19
@openshift-ci openshift-ci bot added the approved label Dec 9, 2024
@stuggi stuggi requested a review from fmount December 9, 2024 06:37
@stuggi
Copy link
Contributor Author

stuggi commented Dec 9, 2024

@fmount there is an issue with the glance crd which prevents bumping to controller tools 0.14.0. The imageCache.size is a required value. With controller-tools 0.14.0 support for empty maps/lists was introduced https://github.com/kubernetes-sigs/controller-tools/releases/tag/v0.14.0 . This now really applies the defaulting in [1] . Is the imageCache config a required config for glance, or can it be optional and we remove [1], so that we can bump to controller-tools 0.14.0?

[1] https://github.com/openstack-k8s-operators/glance-operator/blob/main/api/v1beta1/glance_types.go#L110C5-L110C28

@stuggi
Copy link
Contributor Author

stuggi commented Dec 9, 2024

This is the error:

  [FAILED] Unexpected error:
      <*fmt.wrapError | 0xc0001ba1a0>: 
      unable to install CRDs onto control plane: unable to create CRD instances: unable to create CRD "glances.glance.openstack.org": CustomResourceDefinition.apiextensions.k8s.io "glances.glance.openstack.org" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[imageCache].default.size: Required value
      {
          msg: "unable to install CRDs onto control plane: unable to create CRD instances: unable to create CRD \"glances.glance.openstack.org\": CustomResourceDefinition.apiextensions.k8s.io \"glances.glance.openstack.org\" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[imageCache].default.size: Required value",
          err: <*fmt.wrapError | 0xc0001ba180>{
              msg: "unable to create CRD instances: unable to create CRD \"glances.glance.openstack.org\": CustomResourceDefinition.apiextensions.k8s.io \"glances.glance.openstack.org\" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[imageCache].default.size: Required value",
              err: <*fmt.wrapError | 0xc0001ba160>{
                  msg: "unable to create CRD \"glances.glance.openstack.org\": CustomResourceDefinition.apiextensions.k8s.io \"glances.glance.openstack.org\" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[imageCache].default.size: Required value",
                  err: <*errors.StatusError | 0xc0003a2280>{
                      ErrStatus: {
                          TypeMeta: {Kind: "", APIVersion: ""},
                          ListMeta: {
                              SelfLink: "",
                              ResourceVersion: "",
                              Continue: "",
                              RemainingItemCount: nil,
                          },
                          Status: "Failure",
                          Message: "CustomResourceDefinition.apiextensions.k8s.io \"glances.glance.openstack.org\" is invalid: spec.validation.openAPIV3Schema.properties[spec].properties[imageCache].default.size: Required value",
                          Reason: "Invalid",
                          Details: {
                              Name: "glances.glance.openstack.org",
                              Group: "apiextensions.k8s.io",
                              Kind: "CustomResourceDefinition",
                              UID: "",
                              Causes: [
                                  {
                                      Type: "FieldValueRequired",
                                      Message: "Required value",
                                      Field: "spec.validation.openAPIV3Schema.properties[spec].properties[imageCache].default.size",
                                  },
                              ],
                              RetryAfterSeconds: 0,
                          },
                          Code: 422,
                      },
                  },
              },
          },
      }
  occurred

could be fixed by removing https://github.com/openstack-k8s-operators/glance-operator/blob/main/api/v1beta1/glance_types.go#L110C5-L110C28

@fmount
Copy link
Contributor

fmount commented Dec 9, 2024

@fmount there is an issue with the glance crd which prevents bumping to controller tools 0.14.0. The imageCache.size is a required value. With controller-tools 0.14.0 support for empty maps/lists was introduced https://github.com/kubernetes-sigs/controller-tools/releases/tag/v0.14.0 . This now really applies the defaulting in [1] . Is the imageCache config a required config for glance, or can it be optional and we remove [1], so that we can bump to controller-tools 0.14.0?

[1] https://github.com/openstack-k8s-operators/glance-operator/blob/main/api/v1beta1/glance_types.go#L110C5-L110C28

ImageCache is not required at deployment time: customers can decide to enable this feature later in time (day2) and patch the Glance CR. So I agree this is not a required parameter and the API should be fixed to reflect that.
Thank you, I guess you can remove [1].

[1] https://github.com/openstack-k8s-operators/glance-operator/blob/main/api/v1beta1/glance_types.go#L110

@stuggi
Copy link
Contributor Author

stuggi commented Dec 9, 2024

@fmount there is an issue with the glance crd which prevents bumping to controller tools 0.14.0. The imageCache.size is a required value. With controller-tools 0.14.0 support for empty maps/lists was introduced https://github.com/kubernetes-sigs/controller-tools/releases/tag/v0.14.0 . This now really applies the defaulting in [1] . Is the imageCache config a required config for glance, or can it be optional and we remove [1], so that we can bump to controller-tools 0.14.0?
[1] https://github.com/openstack-k8s-operators/glance-operator/blob/main/api/v1beta1/glance_types.go#L110C5-L110C28

ImageCache is not required at deployment time: customers can decide to enable this feature later in time (day2) and patch the Glance CR. So I agree this is not a required parameter and the API should be fixed to reflect that. Thank you, I guess you can remove [1].

[1] https://github.com/openstack-k8s-operators/glance-operator/blob/main/api/v1beta1/glance_types.go#L110

Ack, was guessing that, but first wanted to check with you. Thanks, will update the pr.

@stuggi
Bump CONTROLLER_TOOLS_VERSION to 0.14.0
614a05d 
bump controller tools to the version which bumps k8s deps to
v0.29.0, which aligns with the version in the operators.

Signed-off-by: Martin Schuppert <[email protected]>
@stuggi stuggi force-pushed the bump_controller_tools branch from a56a6cf to 614a05d Compare December 9, 2024 07:26
@fmount
Copy link
Contributor

fmount commented Dec 9, 2024

@stuggi seems good now and kuttl (which is supposed to validate imageCache) seems to work.

@stuggi
Copy link
Contributor Author

stuggi commented Dec 9, 2024

@stuggi seems good now and kuttl (which is supposed to validate imageCache) seems to work.

yes seems to be good. can you approve ?

fmount
fmount approved these changes Dec 9, 2024
View reviewed changes
Copy link
Contributor

@fmount fmount left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Dec 9, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Dec 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fmount, stuggi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@stuggi
Copy link
Contributor Author

stuggi commented Dec 9, 2024

/retest

@openshift-merge-bot openshift-merge-bot bot merged commit 33b0fc2 into openstack-k8s-operators:main Dec 9, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fmount fmount fmount approved these changes

@abays abays Awaiting requested review from abays

@dprince dprince Awaiting requested review from dprince

Assignees

@fmount fmount

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@stuggi @fmount